Support SELinux Check #386
Conversation
Mashimiao
commented
May 22, 2017
Mashimiao
commented
- update godeps and add go-selinux deps
- add selinux check in validate and runtimetest
Mashimiao
force-pushed
the
support-selinux-check
branch
from
5f18176
to
a5da9b7
Compare
cmd/runtimetest/main.go
Outdated
| return fmt.Errorf("Failed to get mountLabel of %v", mount.Destination) | ||
| } | ||
| if fileLabel != spec.Linux.MountLabel { | ||
| return fmt.Errorf("Expected mountLabel %v, actual %v", spec.Linux.MountLabel, fileLabel) |
There was a problem hiding this comment.
There are no RFC 2119 requirements for the runtime for linux.mountLabel. I think we need to get spec language about what the runtime MUST do before we can fail a runtime for not doing it (opencontainers/runtime-spec#746).
| @@ -519,6 +520,12 @@ func (v *Validator) CheckLinux() (msgs []string) { | |||
| } | |||
| } | |||
|
|
|||
| if v.spec.Linux.MountLabel != "" { | |||
| if err := label.Validate(v.spec.Linux.MountLabel); err != nil { | |||
There was a problem hiding this comment.
I'm not sure what label.Validate is checking, but the spec places no limits on the linux.mountLabel value. If we want to validate it (against a particular kernel API?), then I think we probably need clarity in the spec about what API the value will be passed to, and the check here should probably only happen when HostSpecific is set.
There was a problem hiding this comment.
The validation of mountLabel is not host-specific. It just checks whether label contains unexpected options. Please refer to label.Validate()
There was a problem hiding this comment.
It just checks whether label contains unexpected options.
But there is nothing in the spec about “MUST be a valid SELinux label according to [some SELinux spec]” or “MUST NOT contain z or Z” or anything like that. Currently spec-readers have to make that jump on their own.
And I was suggesting host-specific because even if z and Z are not legal labels for today's kernel, are we sure that they will be illegal labels for all kernels? Maybe someone compiles a Linux kernel where they are legal, and the spec's kernel-punt-where-possible approach is designed to support that sort of thing.
There was a problem hiding this comment.
I still don't see spec grounds for this check, but if the runtime-tools maintainers feel like it's grounded (or if they just want “unlikely to be supported by the kernel” warnings), then the validate/ changes from this PR belong in v0.3.0. That could either happen by spinning them out into a new PR, or by reviewing them here in the context of the runtimetest changes.
Mashimiao
force-pushed
the
support-selinux-check
branch
from
a5da9b7
to
aaa22d0
Compare
|
LGTM |
Mashimiao
force-pushed
the
support-selinux-check
branch
2 times, most recently
from
b165d15
to
36dfee9
Compare
Mashimiao
force-pushed
the
support-selinux-check
branch
from
36dfee9
to
a538e0c
Compare
Signed-off-by: Ma Shimiao <mashimiao.fnst@cn.fujitsu.com>
Signed-off-by: Ma Shimiao <mashimiao.fnst@cn.fujitsu.com>
cmd/runtimetest/main.go
Outdated
| if err != nil { | ||
| return fmt.Errorf("Failed to get mountLabel of %v", mount.Destination) | ||
| } | ||
| if fileLabel != spec.Linux.MountLabel { |
There was a problem hiding this comment.
Linux judgment should be increased to avoid panic.
Mashimiao
force-pushed
the
support-selinux-check
branch
from
a538e0c
to
38713d1
Compare
Signed-off-by: Ma Shimiao <mashimiao.fnst@cn.fujitsu.com>
|
LGTM |
